Add testProcess{Config} task to accept user options by zhengyu123 · Pull Request #581 · DataDog/java-profiler

zhengyu123 · 2026-06-05T19:28:19Z

What does this PR do?:
This PR adds a new task for testing, that accepts user options.

Motivation:
The task is created for testing new feature that avoids preloading all jmethod ids. By default, the feature is disabled and has to be enabled via fjmethodid=false argument, which is not possible with current testdebug/release tasks.

Additional Notes:

How to test the change?:
Run ./gradlew Pprofiler.options=fjmethodid=false testProcessDebug, fjmethodid=false is merged into command line that print out by tests.

For example:

[TEST] Applied profiler.options: fjmethodid=false
===> keep_jfrs: false
com.datadoghq.profiler.wallclock.SmokeWallTest > test PASSED (789ms)
com.datadoghq.profiler.wallclock.SmokeWallTest > test STARTED
[TEST] Applied profiler.options: fjmethodid=false
===> command: start,wall=1ms,fjmethodid=false,cstack=vm,attributes=tag1;tag2;tag3,jfr,file=/tmp/recordings/fp/SmokeWallTest_test14542021396455448632.jfr

For Datadog employees:

If this PR touches code that signs or publishes builds or packages, or handles
credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
This PR doesn't touch any of that.
JIRA: PROF-14940

Unsure? Have a question? Request a review!

…ptions argument to tests

dd-octo-sts · 2026-06-05T19:45:34Z

CI Test Results

Run: #27158474248 | Commit: 912face | Duration: 12m 7s (longest job)

✅ All 32 test jobs passed

Status Overview

JDK	glibc-aarch64/debug	glibc-amd64/debug	musl-aarch64/debug	musl-amd64/debug
8	-	✅	-	-
8-ibm	-	✅	-	-
8-j9	✅	✅	-	-
8-librca	-	-	✅	✅
8-orcl	-	✅	-	-
11	-	✅	-	-
11-j9	✅	✅	-	-
11-librca	-	-	✅	✅
17	✅	✅	-	-
17-graal	✅	✅	-	-
17-j9	✅	✅	-	-
17-librca	-	-	✅	✅
21	✅	✅	-	-
21-graal	✅	✅	-	-
21-librca	-	-	✅	✅
25	✅	✅	-	-
25-graal	✅	✅	-	-
25-librca	-	-	✅	✅

Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled

Summary: Total: 32 | Passed: 32 | Failed: 0

Updated: 2026-06-08 18:44:52 UTC

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6f5c7bae36

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-06-05T20:27:35Z

+    String userOptions = System.getProperty("ddprof.test.options");
+    if (userOptions != null && !userOptions.isEmpty()) {
+      profilerCommand = mergeProfilerOptions(profilerCommand, userOptions);


Apply profiler options to all profiler starts

When testProcessDebug is run without -Ptests, this system property is only consumed by AbstractProfilerTest; tests that call JavaProfiler.execute(...) directly still start the profiler with their hard-coded commands (for example ShutdownTest.runTest and OtelContextStorageModeTest). That means a process run requested with -Pprofiler.options=fjmethodid=false still exercises part of the suite with the default profiler options, so it is not a reliable way to validate that option across the task's full test set.

Useful? React with 👍 / 👎.

jbachorik

Sphinx automated review — PR #581 · zgu/test_cmd_opts

jbachorik · 2026-06-08T14:25:32Z

+    }
+
+    // Apply overrides
+    for (String part : overrides.split(",")) {


🔵 LOW · robustness [GENERALIST]

A valueless override token (e.g. -Pprofiler.options=cpu) silently clears the value of a matching base option: cpu=10ms in the base becomes bare cpu in the merged result. Easy to trigger accidentally and undocumented.

Suggestion: Document this behavior in the mergeProfilerOptions Javadoc so users know that a bare key override strips any existing value. Alternatively, use putIfAbsent to avoid overwriting an existing valued key with an empty one:

else if (!part.isEmpty()) { options.putIfAbsent(part, ""); }

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a13c32a888

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-06-08T18:31:08Z

+      // Skip key with empty value
+      if (!entry.getValue().isEmpty()) {


Preserve empty-valued profiler options

When testProcessDebug is run with any -Pprofiler.options, the merge path rebuilds every base command and drops entries whose value is empty. That changes existing tests such as VirtualThreadWallClockTest, whose base command is wall=1ms,filter=; adding an unrelated override like -Pprofiler.options=fjmethodid=false silently removes filter= (and can leave a doubled comma), so the new task no longer runs the same profiler configuration plus the requested override.

Useful? React with 👍 / 👎.

zhengyu123 added 2 commits June 5, 2026 14:18

Create testProcess{Config} task that can pass command line profiler.o…

42d4d54

…ptions argument to tests

Cleanup

6f5c7ba

zhengyu123 changed the title ~~Zgu/test cmd opts~~ Add testProcess{Config} task to accept user options Jun 5, 2026

zhengyu123 marked this pull request as ready for review June 5, 2026 20:24

zhengyu123 requested a review from a team as a code owner June 5, 2026 20:24

chatgpt-codex-connector Bot reviewed Jun 5, 2026

View reviewed changes

jbachorik reviewed Jun 8, 2026

View reviewed changes

Fix

a13c32a

chatgpt-codex-connector Bot reviewed Jun 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add testProcess{Config} task to accept user options#581

Add testProcess{Config} task to accept user options#581
zhengyu123 wants to merge 3 commits into
mainfrom
zgu/test_cmd_opts

zhengyu123 commented Jun 5, 2026 •

edited

Loading

Uh oh!

dd-octo-sts Bot commented Jun 5, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jun 5, 2026

Uh oh!

jbachorik left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jbachorik Jun 8, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jun 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// Skip key with empty value
		if (!entry.getValue().isEmpty()) {

Conversation

zhengyu123 commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dd-octo-sts Bot commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI Test Results

Status Overview

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

jbachorik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jbachorik Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zhengyu123 commented Jun 5, 2026 •

edited

Loading

dd-octo-sts Bot commented Jun 5, 2026 •

edited

Loading